Skip to content

[java] Add header while creating websocket #15655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Apr 23, 2025
Merged

[java] Add header while creating websocket #15655

merged 5 commits into from
Apr 23, 2025

Conversation

@pujagani
Copy link
Contributor

@pujagani pujagani commented Apr 22, 2025 •
edited by qodo-merge-pro bot
Loading

User description

🔗 Related Issues

Fixes #15645

💥 What does this PR do?

Adds header while building the websocket.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Bug fix (backwards compatible)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR Type

Bug fix, Enhancement

Description

  • Add HTTP headers when creating WebSocket connections

  • Refactor WebSocket builder to include request headers

Changes walkthrough 📝

Relevant files
Enhancement
JdkHttpClient.java
Add request headers to WebSocket builder and connection   

java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java

  • Create WebSocket builder and add request headers as HTTP headers
  • Refactor to use the builder with headers for WebSocket connection
  • Ensure all request headers are included in WebSocket handshake
    		• +5/-2     
    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @selenium-ci selenium-ci added the C-java Java Bindings label Apr 22, 2025
    @pujagani pujagani marked this pull request as ready for review April 23, 2025 09:59
    @qodo-merge-pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ❌

    1234 - Not compliant

    Non-compliant requirements:

    • Fix issue where JavaScript in link's href is not triggered on click() in Firefox 42.0

    Requires further human verification:

    • Need to verify if adding WebSocket headers addresses the JavaScript trigger issue in Firefox

    5678 - Not compliant

    Non-compliant requirements:

    • Fix "Error: ConnectFailure (Connection refused)" when instantiating ChromeDriver

    Requires further human verification:

    • Need to verify if adding WebSocket headers addresses the ChromeDriver connection issues

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Header Validation

    The implementation adds all headers from the request to the WebSocket connection without any validation. Some headers might not be appropriate for WebSocket connections and could cause issues.

     
    request.getHeaderNames().forEach(name -> builder.header(name, request.getHeader(name)));

    @qodo-merge-pro
    Copy link
    Contributor

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @titusfortner titusfortner merged commit 6914339 into SeleniumHQ:trunk Apr 23, 2025
    34 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    No reviews

    Assignees

    No one assigned

    Labels

    C-java Java Bindings Review effort 2/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    [🐛 Bug]: WebSocket headers not applied in JdkHttpClient.openSocket – breaks DevTools behind authenticated proxy

    3 participants

    @pujagani @titusfortner @selenium-ci